Multi-Cluster: Adds v1alpha1 API Types and Docs #1658

danehans · 2025-09-26T17:45:02Z

What type of PR is this?
/kind documentation
/kind feature

What this PR does / why we need it:

Adds the v1alpha1 Go types for multi-cluster inferencepool.
Runs the generators.
Adds the API docs.

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?:

Adds initial Go types and API docs for v1alpha1 multi-cluster InferencePools.

netlify · 2025-09-26T17:45:12Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`754b64b`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/68e43aae2d891c000813ca98
😎 Deploy Preview	https://deploy-preview-1658--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

danehans · 2025-09-26T17:45:21Z

Hold until #1374 is merged.

/hold

danehans · 2025-09-26T17:45:32Z

cc: @robscott @bexxmodd

keithmattix · 2025-09-26T19:02:30Z

api/v1/inferencepool_types.go

+	//
+	// Controllers MAY raise this condition with other reasons, but should
+	// prefer to use the reasons listed above to improve interoperability.
+	InferencePoolConditionExported InferencePoolConditionType = "Exported"


What's the value for a non-exported InferencePool? "Invalid" doesn't sound right IMO

Yeah +1, Added some suggested reasons above.

Here are a few other options we can take:

Do not define the reason at this time and leave it up to the implementation to surface.

Rename "Invalid" to "Unknown" or "ApplyFailed" and require a message to be set with additional details.

Proceed with https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/1658/files#r2383650354 recommendations.

WDYT?

Commit f0fbeb9 proceeds with option 3 above.

I like the recommendations in 3. If the InferencePool has not been exported, then that should be clear

@keithmattix commit f0fbeb9 proceeds with option 3, PTAL.

apix/v1alpha1/inferencepoolimport_types.go

robscott

Thanks @danehans! This mostly LGTM. I'm going to be OOO for the next few days but please feel free to go ahead and move forward with this once my comments are resolved. @bexxmodd can also help with the reviews here. I'm sure we'll still have some things to tweak after we merge this, but I'd rather get something in soon so we can start on the implementation side.

api/v1/inferencepool_types.go

robscott · 2025-09-26T23:14:20Z

api/v1/inferencepool_types.go

+	//
+	// Controllers MAY raise this condition with other reasons, but should
+	// prefer to use the reasons listed above to improve interoperability.
+	InferencePoolConditionExported InferencePoolConditionType = "Exported"


Yeah +1, Added some suggested reasons above.

apix/v1alpha1/inferencepoolimport_types.go

bexxmodd · 2025-09-29T23:59:33Z

apix/v1alpha1/inferencepoolimport_types.go

+}
+
+// InferencePoolImportStatus defines the observed state of the InferencePoolImport.
+type InferencePoolImportStatus struct {


Can we have some field like this in here? https://github.com/kubernetes-sigs/gateway-api/blob/530c1ee46e0d851d369810e4500f0339c40d8aa1/apis/v1/gateway_types.go#L1022

This can be useful for some implementation specific metadata propagation.

@bexxmodd adding ^ would modify the proposal. I can potentially see differing views on how this metadata should be propagated. If this important to you, can you create an issue or link it to here?

SG. I'll create a new issue.

@bexxmodd when you have a moment, please link the issue here and resolve this conversation.

/link #1674

keithmattix

/lgtm - thanks Daneyon!

keithmattix · 2025-09-30T20:54:29Z

/lgtm

bexxmodd · 2025-10-03T16:49:28Z

/LGTM /Approve thanks @danehans

danehans · 2025-10-03T22:44:15Z

Tagging since the latest commit resolves @robscott feedback and multiple reviewers tagged.

/lgtm

k8s-ci-robot · 2025-10-03T22:44:17Z

@danehans: you cannot LGTM your own PR.

In response to this:

Tagging since the latest commit resolves @robscott feedback and multiple reviewers tagged.

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

robscott · 2025-10-03T23:40:07Z

Thanks @danehans!

/lgtm
/hold cancel

kfswain · 2025-10-03T22:51:18Z

api/v1/inferencepool_types.go

+	//
+	// +required
+	//nolint:kubeapilinter // should not have omitempty since the field is required
+	ControllerName ControllerName `json:"controllerName"`


Can we make these changes in a v1alpha3 inferencepool? I'm concerned about fullsending changes to v1 on the first iteration.

Cc: @robscott @ahg-g

We should make this field optional so the change is backwards-compatible, but other than that, there's plenty of precedence for this field existing in GW API statuses. It was an accidental omission from the v1 API.

As far as the mechanics of API versions - we'd actually have to create v2alpha1 if we wanted to make breaking changes. In Gateway API, we created an "experimental" channel that allows us to gate fields like this by only including them in experimental CRDs (conceptually similar to feature gates in k/k). While I think that's inevitable here, I personally don't think this change is large enough to justify creating the separate release channel.

robscott · 2025-10-03T23:48:01Z

/hold

robscott

Thanks @danehans!

robscott · 2025-10-04T00:44:39Z

api/v1/inferencepool_types.go

+	//
+	// +required
+	//nolint:kubeapilinter // should not have omitempty since the field is required
+	ControllerName ControllerName `json:"controllerName"`


We should make this field optional so the change is backwards-compatible, but other than that, there's plenty of precedence for this field existing in GW API statuses. It was an accidental omission from the v1 API.

As far as the mechanics of API versions - we'd actually have to create v2alpha1 if we wanted to make breaking changes. In Gateway API, we created an "experimental" channel that allows us to gate fields like this by only including them in experimental CRDs (conceptually similar to feature gates in k/k). While I think that's inevitable here, I personally don't think this change is large enough to justify creating the separate release channel.

kfswain · 2025-10-06T00:24:50Z

/lgtm

I think we should address:

We should make this field optional so the change is backwards-compatible

But otherwise everything sounds good

Signed-off-by: Daneyon Hansen <[email protected]>

danehans · 2025-10-06T21:55:52Z

754b64b makes ControllerName optional and updates the godocs for this field.

robscott

Thanks @danehans!

robscott · 2025-10-07T00:09:17Z

apix/v1alpha1/inferencepoolimport_types.go

+	// Name of the exporting cluster (must be unique within the list).
+	//
+	// +kubebuilder:validation:Required
+	Name ClusterName `json:"name"`


Was chatting with @bexxmodd about this earlier today and he pointed out that we likely also need some basic information about the EPP in each cluster here. I think this can/should be an additive change, so don't want to block this PR for this, just raising it as an addition that would be helpful.

It could also be useful to store some per-cluster metadata beyond name, maybe conceptually similar to the infrastructure field we have on Gateway. Again, not a blocker, but probably worth a follow up PR if @bexxmodd can squeeze it in.

k8s-ci-robot · 2025-10-07T00:10:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bexxmodd, danehans, robscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [danehans]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kfswain · 2025-10-07T00:16:14Z

/unhold

Thanks!

robscott · 2025-10-07T05:06:53Z

/lgtm

k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 26, 2025

k8s-ci-robot requested review from kfswain and robscott September 26, 2025 17:45

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2025

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 26, 2025

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 26, 2025

keithmattix reviewed Sep 26, 2025

View reviewed changes

robscott approved these changes Sep 27, 2025

View reviewed changes

danehans force-pushed the issue_1654 branch 2 times, most recently from f0fbeb9 to ac3dc45 Compare September 29, 2025 23:21

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2025

bexxmodd reviewed Sep 29, 2025

View reviewed changes

danehans force-pushed the issue_1654 branch from ac3dc45 to b048b79 Compare September 30, 2025 18:25

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2025

keithmattix reviewed Sep 30, 2025

View reviewed changes

k8s-ci-robot assigned keithmattix Sep 30, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2025

danehans force-pushed the issue_1654 branch from b048b79 to dbe103f Compare October 2, 2025 16:05

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 2, 2025

bexxmodd approved these changes Oct 3, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2025

k8s-ci-robot assigned robscott Oct 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2025

kfswain reviewed Oct 3, 2025

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2025

robscott reviewed Oct 4, 2025

View reviewed changes

k8s-ci-robot assigned kfswain Oct 6, 2025

danehans added 3 commits October 6, 2025 14:44

Multi-Cluster: Adds v1alpha1 API Types and Docs

9570482

Signed-off-by: Daneyon Hansen <[email protected]>

Resolves initial review comments

bfa2326

Signed-off-by: Daneyon Hansen <[email protected]>

Makes ControllerName optional

754b64b

Signed-off-by: Daneyon Hansen <[email protected]>

danehans force-pushed the issue_1654 branch from dbe103f to 754b64b Compare October 6, 2025 21:54

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2025

robscott approved these changes Oct 7, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2025

k8s-ci-robot merged commit 67ad182 into kubernetes-sigs:main Oct 7, 2025
11 checks passed

Multi-Cluster: Adds v1alpha1 API Types and Docs #1658

Multi-Cluster: Adds v1alpha1 API Types and Docs #1658

Conversation

danehans commented Sep 26, 2025

Uh oh!

netlify bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

danehans commented Sep 26, 2025

Uh oh!

danehans commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danehans Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithmattix left a comment

Choose a reason for hiding this comment

Uh oh!

keithmattix commented Sep 30, 2025

Uh oh!

bexxmodd commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danehans commented Oct 3, 2025

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

robscott commented Oct 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robscott commented Oct 3, 2025

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfswain commented Oct 6, 2025

Uh oh!

danehans commented Oct 6, 2025

netlify bot commented Sep 26, 2025 •

edited

Loading

danehans Oct 2, 2025 •

edited

Loading

bexxmodd commented Oct 3, 2025 •

edited

Loading